Skip to content

fix(aggregation): recover from over-budget cost estimates - #422

Merged
shaaibu7 merged 22 commits into
fix/aggregation-skip-visibilityfrom
fix/aggregation-budget-recovery
Sep 9, 2026
Merged

fix(aggregation): recover from over-budget cost estimates#422
shaaibu7 merged 22 commits into
fix/aggregation-skip-visibilityfrom
fix/aggregation-budget-recovery

Conversation

@dimka90

@dimka90 dimka90 commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Prevent the aggregation worker's persistent per-group cost estimate from locking out every subsequent session after an over-budget observation.

Stacked on #419 (fix/aggregation-skip-visibility); retarget after that PR merges.

Root cause

The worker retains one estimator across sessions. Admission previously compared the estimate against the remaining budget even before the first proof. An estimate above the entire session budget therefore rejected all subsequent sessions, and no successful proof could supply a recovery sample.

Changes

  • Allow the first actual proof attempt while the session deadline is still live. Groups skipped during preparation do not consume this allowance; failed proof attempts do.
  • Keep estimated-cost admission for subsequent attempts, without clamping observations.
  • Recheck the actual deadline after preparation and before entering the prover.
  • Count prover errors using the existing error skip label.
  • Add deterministic scheduling tests with an injected prover and fake time. Cover an initial slow sample, an EMA spike, repeated recovery sessions, failed attempts, expired deadlines, overruns, and partial-result/signature retention. These tests failed with the original admission condition and pass with the fix; they do not test cryptographic validity.

Spec and scope

Pinned leanSpec: eca701efeb5931010fe63925cd203c9ee55b2dbc, AggregationMixin.aggregate in src/lean_spec/spec/forks/lstar/aggregation.py, mapped to gean's aggregateFromSnapshot. The spec has no deadline or persistent cost estimator; this change repairs gean-specific scheduling. No wire format, verification rules, signature selection policy, dependency pins, or concurrency ownership changes.

The observed gean_15 logs show repeated produced=0 skipped=budget=1 sessions completing in milliseconds. They support the refusal symptom but do not establish the original slow proof or the deployed commit.

Validation

  • make fmt
  • make build
  • make test
  • go test ./internal/aggregation ./internal/metrics -count=1
  • go test -race ./internal/aggregation -count=1
  • make lint
  • git diff --check and scoped correctness/simplification review
  • make test-spec — pinned fixture generation is still running; not claimed passed
  • Local runtime validation — in progress by the author
  • Eight-subnet, single-subnet-per-aggregator multi-client validation

Risks and follow-up

An in-flight proof is not cancellable by this deadline and can still overrun, delaying other gate users. Input sizing is an estimate, not a hard runtime bound. Keep this PR draft pending runtime validation.

Counting all deferred groups rather than one budget-break event changes the metric's semantics and is intentionally left for a separate change. Backlog pruning is untouched.

Three comments described behaviour the code does not have.

worker.go said groups are proven newest-first; orderedGroups sorts by
ascending target slot, deliberately, so a short budget is spent on the
lowest unjustified targets and finalization keeps moving.

tick.go said the proving gate's proposal priority handles contention with
an upcoming proposal duty. The priority flag only blocks the next
background Acquire; a session already holding the token runs to
completion, so a proposal landing mid-session waits for the whole
session. Naming the per-session group cap as the real bound keeps the
next reader from trusting a guarantee that is not there.

validate.go said admission mirrors the prune predicate. Admission tests
head slot plus ancestry from finalized, which is leanSpec's rule;
PruneBelow tests Data.Slot for signatures and Target.Slot for payloads.
They are three different predicates.

Comments only; no behaviour change.
Two counters described a session more kindly than it deserved.

IncProofOperation("aggregation", "success") fired on every session that
reached the end of the worker, including one that dropped every group and
published nothing. On devnet-5 an aggregator produced nothing for 355
consecutive slots while the success rate read 100%, which is precisely
the case the counter exists to surface. A session with no output is now
counted as "empty".

A budget stop defers every group still queued, but skips.add recorded a
single "budget" skip because the break follows it immediately. The count
now covers the whole remaining queue, so the skip summary distinguishes a
session that gave up on one group from one that gave up on thirty.

Adds addN to groupSkips and a test asserting a three-group queue stopped
at the deadline reports three deferrals.
Recursive proofs cost 1.68-2.96s against raw-only proofs at 0.31-0.91s,
measured on a 16-core host with no overlap between the two ranges. With
four aggregators on four subnets, 77% of gean's groups were recursive,
sessions overran the budget on essentially every slot, and the node fell
129 slots behind while the chain forked. ethlambda ran the identical
topology on the same host at 23% recursion and stayed 9 slots behind,
finalizing.

The share is not a property of the network. selectChildProofs ran before
any raw signature was considered, so every group holding a usable child
proof became recursive. Seeding coverage from the raw signatures first
leaves most children with nothing to add, and a child that adds nothing
is never selected.

Three changes, one selection pass:

  - Raw signatures claim coverage first; children fill only the gaps.
  - Children are chosen greedily on coverage rather than in pool order.
    Stored order can take several narrow proofs where one wide proof
    covers the same validators, and each extra child is a recursive input
    the prover pays for. Ties break on the participant bitfield so the
    choice is stable run to run.
  - Raw signatures a chosen child already covers are trimmed. This is
    required, not an optimisation: child-first ordering was what
    guaranteed a validator never appeared both as a raw participant and
    inside a child, and reversing the order removes that guarantee.

This diverges from leanSpec's aggregate(), which selects children first
and fills with raw. The output is a single valid aggregate over the same
participants either way, so ordering is an implementation freedom; the
spec models no cost. ethlambda's resolve_job makes the same choice.

Group ordering is untouched: orderedGroups still runs frontier-first by
ascending target slot to keep finalization moving under a short budget.
Raw-first selection removes most recursion, but it does not bound what a
single group can reach for. A group with little local raw coverage still
folds in every child that adds validators, and each one is a recursive
input: 1.68-2.96s measured against 0.31-0.91s for raw-only groups, with
no overlap between the ranges.

Two children is the same value ethlambda and lantern settled on, both
noting recursion as the cost to bound.

The cap counts children already in the slice rather than per call, so it
holds across the separate new-payload and known-payload passes that build
one group's inputs. Selecting from the wider pool first still applies:
greedy coverage means the two children kept are the two that cover most.
A session's cost was whatever the backlog cost. With four aggregators
covering four subnets that saturated a 16-core host: the node fell 129
slots behind, the chain forked one slot after justification stopped, and
essentially every session overran its budget.

The wall-clock budget cannot prevent this on its own. It is checked
between groups, so it bounds when a session stops starting work, not what
a session may cost in total, and the first group always runs. A count is
the cruder bound but the one that holds before any proof begins.

Two groups per session, dropping to one in the slot before this node
proposes. The proving gate gives proposals priority, but priority only
defers the next background acquire: a session already holding the token
runs to completion and the proposal waits for it, so the cap is what
bounds that wait.

The cap counts proof attempts rather than loop iterations, since groups
dropped for a justified target or too few signers never reach the prover
and cost nothing. Groups it defers are reported as session_cap, kept
separate from budget so the skip summary distinguishes "ran out of time"
from "reached the limit".

The proposer lookahead reuses the head state dispatchAggregationCycle had
already decoded, via a new proposingAt helper, rather than calling
getOurProposer and paying a second SSZ decode on the tick loop.
SessionBudget is two intervals measured from the moment the worker
acquires the prover. Its own comment gives the reason as leaving interval
4 free for results to be promoted and gossiped, so the real constraint is
a boundary, not a span. Expressing it as a span is only correct when the
session starts exactly at interval 2.

It does not. Two cases came out wrong:

The early path dispatches in late interval 1 to give the slow proof a
head start. Starting at 0.8s and adding 1.6s put its deadline at 2.4s,
mid-interval 3, when it could safely have run to 3.2s. The head start the
path exists to create was handed straight back.

workerStart is taken after the gate is acquired, so a session that waited
on the prover got a full budget from whenever it won the token. Waiting
700ms then running 1.6s ends at 3.9s, past the promotion the aggregate
was produced for.

The dispatcher now computes the deadline from the slot clock and passes
it on Dispatch; the worker falls back to SessionBudget when it is unset.
Both dispatch paths land on the interval-4 boundary, and gate waiting
comes out of the window instead of extending it. The overrun log reports
the window actually allowed rather than the nominal constant.

The deadline is derived from the tick's own timestamp rather than a fresh
clock read, so it is exactly the boundary and does not drift by however
long the tick took to reach the dispatch.

recovery.go already treats aggregationDispatchOffset + SessionBudget as
the end of the window; that stays equal to the interval-4 boundary.
observe divided a group's wall time by rawCount+childCount, so a child
proof and a raw signature cost the same unit. They do not: a group of ten
signatures ran 0.6s while the same ten plus one child ran 2.3s. Folding
both into one average left the estimate roughly twenty times wrong for
each population, and maxUnitsWithin then sized every group by a figure
that described neither.

The estimator now tracks perRawSeconds and perChildSeconds. A raw-only
group prices the signature directly; a group carrying children attributes
the raw share at the current raw estimate and charges the residual to the
children. That is well conditioned because raw-only groups are the common
case once selection is raw-first. A group cheaper than its raw share
alone teaches nothing about its children and is ignored rather than
driving the estimate negative.

Selection charges a child childUnitCost in raw-signature units instead of
one. Two exemptions keep the price from starving the thing it is meant to
protect:

  - Children admitted while rawCount+children is below two, since the
    group is not yet spec-viable and charging for them could leave it
    unable to produce anything at all.
  - The first child of any group. Raw-first selection means a chosen
    child only ever covers validators no raw signature reaches, so those
    votes have no fallback; pricing that child out under a tight budget
    would defer them every session for as long as the pressure lasts.

Everything after that is charged. Worst case per session is still one
in-flight proof, since the deadline check between groups stops the next
one.
aggregatedPayloadCap and newPayloadCap were both 0, so the FIFO eviction
in PayloadBuffer.Push has never run: it is gated on capacity > 0.
AttestationSignatureMap had no capacity field at all.

That is survivable only while finalization advances, because
PruneOnFinalization is the sole path that clears these three pools and it
runs on finalization alone. PeriodicPrune, the fallback for a stall,
prunes non-canonical states and blocks and touches none of them. So the
one situation that makes the pools grow without limit is also the one
that switches off the only thing that empties them, which is the shape
seen on devnet-5.

Caps are stall insurance rather than an operating limit; a healthy node
never approaches them between prunes. Signatures evict whole data roots
oldest-first rather than individual signatures, so a surviving root still
carries every vote it collected, which is what an aggregate needs.

This does not reduce resident memory. A node at 3.15 GB RSS reported
102 MB held by the Go runtime, so these pools are not where the memory
is; the prover's Rust allocations are. The caps remove a worst case, they
do not move today's number.
Insert appended without checking whether the validator had already voted
for that attestation data, and nothing upstream deduplicated. Gossip
meshes deliver the same attestation more than once, and
replayPendingAttestations re-enters onGossipAttestation for every
buffered vote once its head block arrives, so the same signature was
stored repeatedly.

Two costs. Each duplicate is a SignatureSize array kept for nothing. And
SignatureCountForSlot sums len(entry.Signatures), so a duplicate inflates
it — that count is compared against ceil(2n/3), a threshold over distinct
validators, to decide whether to pull the aggregation session forward.

Verification is the expensive half: an XMSS check costs hundreds of
milliseconds, and a duplicate cannot change its outcome. The store's own
record of what it holds serves as the seen set, so it needs no separate
cache and is pruned along with the signatures.
GetState deserializes the whole state from SSZ on every call, and
dispatchAggregationCycle paid for it twice: once in the guard that only
checked the state was present, then again inside SnapshotInputs. Both on
the tick goroutine, which also imports blocks and updates the head.

SnapshotInputs now takes the state the dispatcher already resolved. With
the proposer lookahead in the session-cap commit reusing the same state,
a dispatch decodes it once.

Deliberately not adding a state cache. States are content-addressed and
immutable in principle, so an LRU keyed by root looks free, but
StateTransition mutates its argument in place and both
blockprocessor.Process and the proposal path run it directly on what
GetState returned. Handing those callers a cached pointer would let them
corrupt the entry for every later reader. A cache needs either
copy-on-read or a separate read-only accessor, which is a change of its
own rather than a line in this one.
selectProofs ran its greedy loop until no proof added coverage, and
handed the whole slice to Merge. Merge builds a proof from children
alone, with no raw signatures to anchor it, so every proof it receives is
a recursive input and the result is the most expensive shape the prover
produces.

It runs on the proposal path, holding the proving gate at priority, where
an overrun delays the block itself. Capping the aggregation session while
leaving this unbounded would move the cost rather than remove it.

Two, matching the per-group cap aggregation now applies. Not attempting
the further heuristic of preferring a single proof when merging adds
little coverage: "little" needs a threshold, and there is no measurement
behind one yet.
maybeEarlyAggregate compares the votes collected for a slot against
ceil(2n/3) of the entire validator registry. A validator votes on the
subnet given by its index modulo the committee count, and a node only
receives the subnets it joined, so an aggregator covering a subset can
never reach that threshold however complete its view of its own subnets
is. The early path then never fires for it, and the proving head start
that path exists to give is never taken.

The threshold is now measured against the validators assigned to the
subnets this node actually subscribes to. With one committee, or an
aggregator subscribed to every subnet, that is the whole registry and
nothing changes; the devnet runs so far have all been in that shape,
which is why this stayed hidden.

AggregateSubnetIDs reached p2p but never the engine, so it is now a field
main sets after New.
Two problems, one cause.

The signature map pruned on Data.Slot while both payload buffers pruned
on Data.Target.Slot, so each kept data roots the other had dropped. The
target is the correct key: it decides whether a vote can still advance
finality, it is what lantern and ethlambda's payload buffer use, and it
is what orderedGroups already reads. A malformed entry without a target
falls back to the attestation slot, matching orderedGroups.

More importantly, all three pools are pruned only by PruneOnFinalization,
which runs when the finalized slot advances. PeriodicPrune, the existing
stall fallback, prunes non-canonical states and blocks and touches none
of them, requires finalization to be more than two pruning intervals
behind, and fires only on exact multiples of that interval, so one
skipped slot costs another interval. The single situation that makes the
pools grow without limit is therefore the situation that switches off
everything that empties them, which is the shape observed on devnet-5:
target_justified skips climbing past 1,600 while nothing pruned.

PruneStaleAttestationPools sweeps against a head-relative cutoff instead,
run each slot at interval 3 alongside PeriodicPrune. It is a no-op below
the finalized slot, so a healthy node never pays for it and
PruneOnFinalization keeps owning that range. Data roots holding an
aggregated payload keep their raw signatures, since those are the
coverage a live aggregate was built from; ream and grandine both protect
their head-relative sweeps the same way.
orderedGroups sorted purely by ascending target slot. The reasoning
behind that is sound and stands: finalization advances only when the
checkpoint after the current source is justified, so a short budget is
best spent on the lowest unjustified targets.

It just is not the whole ordering. This slot's votes are the only ones
with a deadline — they have to be aggregated and gossiped in time to
reach the next block, while a backlog entry loses nothing by waiting a
slot. With a session now capped at two groups, sorting by target alone
would spend both on the oldest backlog and leave the current slot's own
votes unaggregated, every slot, for as long as a backlog exists.

Current-slot groups first, then the existing frontier rule within each
tier. ethlambda's scorer makes the same first cut for the same reason:
the slot's committee aggregate is the one piece of work with a deadline.

This is the smaller half of the change discussed. The full tiering —
Finalize before Justify before Build, scored against a projection of the
head state — can follow; it needs a projection this pass does not build.
Pruning the signature map removed each stale root from the insertion
order individually, rescanning that slice per root. Quadratic exactly
when a sweep has the most to drop, which is the stall case the sweep was
added for. The order is now rebuilt in one pass after the map is
filtered.

Anchoring the deadline to the slot means waiting on the prover, or behind
a session that overran, can consume the whole window before this one
starts. The session then ran with an expired deadline, proved nothing,
and reported "hit budget without output" — the starvation warning that
surfaced the estimator latch, raised for a session that never had time to
begin with. Such a dispatch is now skipped with its own log and result
label, so the starvation alarm keeps meaning what the team reads it to
mean.
Two more found reviewing the branch.

expectedVotersPerSlot counted validators on every call, and it is called
once per verified attestation via the early-aggregation wake-up. That is
a loop over the registry per arrival, sitting directly beside the comment
explaining that numValidators is cached to avoid exactly that. Its inputs
are fixed once the registry is known, so it is now cached the same way.

The stale sweep exempted data roots holding an aggregated payload,
following ream and grandine, on the reasoning that those signatures are
the coverage a live aggregate was built from. That exemption cannot do
anything here: a root's signature entry and its payload entry hold the
same AttestationData, so they carry the same target slot and go stale in
the same sweep. Nothing is ever exempt. Removed the protected set, the
Roots accessor added to serve it, and the ordering constraint it imposed;
a test now asserts the two leave together.
aggregationDeadline subtracted the slot position from the interval-4
offset and only then checked whether the position was past it. These are
unsigned milliseconds, so for a position at or beyond the boundary the
difference wraps to an enormous value and the conversion to a Duration
overflows. The guard overwrote the result, so nothing observable went
wrong, but the ordering is a trap for the next edit.

Handle the out-of-range case first and subtract only where the result is
known to be positive.
The cap counts proofs, and PushData adds entries carrying none, so the
data-only entries block import creates are invisible to it. On devnet-5
most of roughly 2,900 known entries were of that kind. The previous
comment implied the cap bounded the buffer as a whole.

The weighting is right: an entry is an AttestationData of a few hundred
bytes while a proof may reach 512 KiB, so proofs are what bound memory,
and entry growth is bounded by pruning instead. Only the description was
wrong.

Both constants now cite the numbers they were chosen against — roughly
2,300 signatures held between prunes on a devnet-5 aggregator, and a live
proof count in the tens at eight committees — rather than reading as
round numbers with no provenance.
Measured on the 16-core host with four aggregators running, a group of two
raw signatures took 2.0-5.2s and produced ~146 KB of proof. Carrying more
signatures barely moved either figure: the cost is the proof, not what it
covers.

The estimator assumed otherwise. observe divided a group's duration by its
signature count and read the result as a per-signature price, so a 4.2s
two-signature group recorded 2.1s per signature. maxUnitsWithin then
computed 1.6s / 2.1s = 0 and returned its floor of two, and every later
group carried exactly two signatures. The estimate confirmed itself: small
groups look expensive per signature, which keeps the next group small.

The cost of that is paying 2-5s for a proof covering two validators when
the same proof could have covered every signature the group held. Twice
the groups for the same coverage, and the surplus lands in the backlog —
333 groups deferred in a single observed session, with the frontier groups
finalization needs among them.

Raw signatures are no longer rationed. What is rationed is proofs, which is
what actually costs: the per-session group cap and the deadline. Cost is
now modelled as perGroupSeconds + children x perChildSeconds, where a
raw-only group prices the fixed term directly and a group carrying children
charges them whatever that term does not explain.

This removes maxUnitsWithin, childUnitCost, perRawSeconds and
seedPerRawSeconds; observe folds into observeGroup, which already measured
whole-group wall time and was already called. Children keep their price,
charged in wall time against the window rather than in signature-equivalent
units, and keep both exemptions.

Three tests asserted the two-signature floor as expected behaviour and now
assert the full signature set.
… raw

Removing the per-signature budget rests on proof cost being independent of
how much a proof covers. Measured across four aggregators: ~146 KB from
two signatures through four, ~170 KB from five through eleven. A step
function, nearly flat within each step, so eleven signatures cost 16% more
proof than two while carrying five and a half times the coverage.

The distribution also shows what the floor was doing: 2,159 groups at
exactly two signatures against single digits at every other size.

The steps are logarithmic, so a group covering a 512-node network stays
inside the 512 KiB proof ceiling, and past it the prover returns
ErrProofTooBig and the group is skipped rather than failing unsafely.
@dimka90
dimka90 marked this pull request as ready for review September 8, 2026 06:17
@shaaibu7
shaaibu7 merged commit 3be1acd into fix/aggregation-skip-visibility Sep 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants